Skip to content

[SPARK-46515] Add MONTHNAME function#44483

Closed
stefankandic wants to merge 10 commits intoapache:masterfrom
stefankandic:monthname-function
Closed

[SPARK-46515] Add MONTHNAME function#44483
stefankandic wants to merge 10 commits intoapache:masterfrom
stefankandic:monthname-function

Conversation

@stefankandic
Copy link
Contributor

@stefankandic stefankandic commented Dec 25, 2023 •
edited
Loading

What changes were proposed in this pull request?

Added MONTHNAME function which returns three-letter abbreviated month name for a given date to:

  • Scala API
  • Python API
  • R API
  • Spark Connect Scala Client
  • Spark Connect Python Client

Why are the changes needed?

for parity with Snowflake

Does this PR introduce any user-facing change?

Yes, new MONTHNAME function

How was this patch tested?

With newly added unit tests

Was this patch authored or co-authored using generative AI tooling?

No

HyukjinKwon
HyukjinKwon reviewed Dec 26, 2023
View reviewed changes
@stefankandic stefankandic changed the title Monthname function [SPARK-46515] Add MONTHNAME function Dec 26, 2023
@stefankandic stefankandic marked this pull request as ready for review December 26, 2023 17:20
@srielau
Copy link
Contributor

srielau commented Dec 27, 2023 •
edited
Loading

Parity with what?
When I look at mySQL:
https://dev.mysql.com/doc/refman/5.7/en/date-and-time-functions.html#function_monthname
It seems to print out the complete name. It also seems to obey the locale.

Snowflake:
https://docs.snowflake.com/en/sql-reference/functions/monthname
So that's where the three letter acronym originates from.
Snowflake does not specify whether it's English only.
We should have someone test this.

Note that if we start with English, then for non English locale's this would turn into a breaking change when we extend to it. So we need to tread crefully to not create a trap for ourselves.

@MaxGekk
Copy link
Member

MaxGekk commented Jan 5, 2024

+1, LGTM. Merging to master.
Thank you, @stefankandic and @srielau for review.

@MaxGekk MaxGekk closed this in e8dfcd3 Jan 5, 2024
@stefankandic
Copy link
Contributor Author

stefankandic commented Jan 5, 2024

Thanks for the review! I guess it only makes sense to also add support for DAYNAME for consistency purposes? @srielau @MaxGekk

cloud-fan
cloud-fan reviewed Jan 8, 2024
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
usage = "_FUNC_(date) - Returns the three-letter abbreviated month name from the given date.",
examples = """
Examples:
> SELECT _FUNC_('2008-02-20');
Copy link
Contributor

@cloud-fan cloud-fan Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the function doc, shall we avoid implicit cast and use DATE'2008-02-20'?

beliefer
beliefer reviewed Jan 8, 2024
View reviewed changes
Copy link
Contributor

@beliefer beliefer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Late LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cloud-fan cloud-fan cloud-fan left review comments

@HyukjinKwon HyukjinKwon HyukjinKwon left review comments

@beliefer beliefer beliefer left review comments

@MaxGekk MaxGekk MaxGekk approved these changes

+1 more reviewer

@srielau srielau srielau approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CONNECT DOCS PYTHON R SQL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@stefankandic @srielau @MaxGekk @cloud-fan @HyukjinKwon @beliefer